-
Notifications
You must be signed in to change notification settings - Fork 2
[Concept]: feat: introducing lazy chain loading #644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 52648b1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // default - loads all chains from the networks config | ||
| chainSelectorsToLoad := cfg.Networks.ChainSelectors() | ||
|
|
||
| if loadcfg.chainSelectorsToLoad != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer care about what chains to load via chainSelectorsToLoad as we no longer load everything.
2c1b033 to
a5255df
Compare
chain/lazy_blockchains.go
Outdated
| chains := make(map[uint64]evm.Chain) | ||
| var errs []error | ||
|
|
||
| for _, selector := range selectors { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this must be done in parallel for the lazy loading to become a viable option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done! another good pick up, updated the other getters to be parallel too!
| blockChains, err := chains.LoadChains(ctx, lggr, cfg, chainSelectorsToLoad) | ||
| // Use lazy loading for chains - they will be initialized on first access | ||
| // All chains from the network config are made available, but only loaded when accessed | ||
| lazyBlockChains, err := chains.NewLazyBlockChains(ctx, lggr, cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should feature flag this initially. Nothing too complicated, just check for an environment variable -- say, CLD_LAZY_BLOCK_CHAINS=1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, added a feature toggle.
b71011b to
ec50b62
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces lazy loading of blockchain chains as an opt-in feature controlled by the CLD_LAZY_BLOCKCHAINS environment variable. Chains are now loaded on-demand when accessed rather than eagerly at initialization, improving startup performance and allowing users to avoid providing secrets for unused chains.
Key changes:
- Introduces
LazyBlockChainstype that implements on-demand chain loading with caching - Adds
BlockChainCollectioninterface implemented by bothBlockChainsandLazyBlockChains - Updates the
Environmenttype to useBlockChainCollectionfor flexibility - Adds comprehensive test coverage for the new lazy loading functionality
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| chain/lazy_blockchains.go | New implementation of lazy chain loading with thread-safe caching |
| chain/lazy_blockchains_test.go | Comprehensive test suite for lazy loading behavior |
| chain/blockchain.go | Adds BlockChainCollection interface to support both eager and lazy loading |
| engine/cld/chains/chains.go | Adds NewLazyBlockChains function and ChainLoader type alias |
| engine/cld/environment/environment.go | Adds lazy loading path with feature flag check |
| engine/cld/environment/environment_test.go | Tests for both lazy and eager loading modes |
| deployment/environment.go | Updates Environment type to use BlockChainCollection interface |
| engine/test/environment/environment_test.go | Updates type references to BlockChainCollection |
| engine/cld/legacy/cli/mcmsv2/mcms_v2.go | Updates type reference to BlockChainCollection |
| experimental/analyzer/evm_analyzer_test.go | Updates test fixtures to use empty BlockChains instances |
| .changeset/empty-words-tickle.md | Changeset documenting the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ec50b62 to
3ee45b9
Compare
Chains are loaded lazily instead of eager, this means chains will only be loaded when it is being used, this means users are not forced to provide any secrets for chains if they are not used upfront and avoid loading huge amount of chains that are never used. This change also means we can deprecate `ChainOverrides` feature as we no longer have to tell CLD what chains to load. This lazy loading is hidden under the feature flag CLD_LAZY_BLOCKCHAINS
3ee45b9 to
52648b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Chains can now be loaded lazily instead of eager, this means chains will only be loaded when it is being used. Hidden under the feature toggle `CLD_LAZY_BLOCKCHAINS` , off by default. Basically this solves a bunch of issues - avoid loading all the chains in the network config file even they are not used (increases loading time) - failures on loading certain chains that are not used will now not failed the changeset - users having to always specify chain overrides in input yaml in order to avoid loading all the chains (this can be tricky if the chain overrides is a long list of chains too) - basically deprecate the `ChainsOverrides` feature, simplifying the code


Chains can now be loaded lazily instead of eager, this means chains will only be loaded when it is being used.
Hidden under the feature toggle
CLD_LAZY_BLOCKCHAINS, off by default.Basically this solves a bunch of issues
ChainsOverridesfeature, simplifying the code